Skip to content

[ZEPPELIN-6758] Fix notebook-scoped AngularObject binding in New UI - #5594

Open
miinhho wants to merge 8 commits into
apache:masterfrom
miinhho:ZEPPELIN-6758-notebook-angular-object
Open

[ZEPPELIN-6758] Fix notebook-scoped AngularObject binding in New UI#5594
miinhho wants to merge 8 commits into
apache:masterfrom
miinhho:ZEPPELIN-6758-notebook-angular-object

Conversation

@miinhho

@miinhho miinhho commented Oct 28, 2026

Copy link
Copy Markdown
Contributor

What is this PR for?

This PR fixes notebook-scoped AngularModel values not appearing in %angular output in the Angular-based New UI.

The server sends notebook-scoped updates without a paragraphId. The existing consumer only accepts updates whose paragraphId matches the current paragraph, so it discards these updates even though both paragraphs finish successfully.

The fix:

  • Applies updates and removals according to their notebook and paragraph scopes.
  • Retains values received before output creation and hydrates newly created or recreated output contexts.
  • Preserves the original scope and interpreter group when updating an existing model from the UI.
  • Cleans up retained bindings when paragraphs or published views are removed.
  • Reapplies server values after template-local edits, even when the incoming value matches the cache.
  • Preserves retained values when loading revisions that do not replay the Angular object registry.
  • Aligns SDK scope types with server payloads and includes interpreter-group identity in interpreter-originated removal messages.

What type of PR is it?

Bug Fix

What is the Jira issue?

How should this be tested?

From zeppelin-web-angular, run the affected consumer and SDK tests:

npm run test:shell -- \
src/app/services/ng-z.service.spec.ts \
src/app/services/runtime-compiler.service.spec.ts \
src/app/core/paragraph-base/paragraph-base.spec.ts \
src/app/pages/workspace/notebook/notebook.component.spec.ts \
src/app/pages/workspace/published/paragraph/paragraph.component.spec.ts \
src/app/pages/workspace/share/result/result.component.spec.ts \
projects/zeppelin-sdk/src/angular-object-values.spec.ts \
projects/zeppelin-sdk/src/interfaces/notebook-wire-fields.spec.ts

Check the SDK contracts and build the Angular application:

npm run typecheck:sdk-contracts
npm run build:angular -- --configuration development

For the server-side removal change, run NotebookServerTest#interpreterRemovalPreservesGroupAndRoutesAllScopes using the project's server-test setup. It checks interpreter-group metadata and notebook/global removal routing.

Manual testing

Use the Angular-based New UI with a working Spark interpreter.

  1. Create a notebook-scoped model:
%spark
import org.apache.zeppelin.display.angular.notebookscope.AngularModel

val controlValue = AngularModel("controlValue", "hello")
  1. Run a separate paragraph:
%angular
<div>value={{controlValue}}</div>

Verify that the output displays value=hello.
3. Update and then remove the model from Spark:

%spark
controlValue() = "updated"
%spark
controlValue.remove()

Verify that the output changes to value=updated and then clears the value.

Scope isolation, native UI-to-server updates, output recreation, and navigation cases are covered by the regression tests listed above.

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

@miinhho
miinhho marked this pull request as draft October 28, 2026 13:33
@miinhho
[ZEPPELIN-6758] Add E2E coverage for notebook-scoped Angular objects
917eaf9 
Use the Python interpreter to verify initial values received before
output creation, server-to-UI updates, native Angular edits reaching the
original server model, and object removal.
@miinhho
miinhho marked this pull request as ready for review October 28, 2026 17:46
@voidmatcha

voidmatcha commented Nov 1, 2026

Copy link
Copy Markdown
Member

Each paragraph receives the same server message and then refreshes all paragraphs again, resulting in 400 context-change events for 20 paragraphs. Please refer to this proposal branch, which changes server updates to refresh only the receiving paragraph.

On the proposal branch, I verified initial value display, bidirectional updates, and binding removal against a real server.

@miinhho

miinhho commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Fixed two additional issues:

  • A model named releaseContext could collide with the output component’s cleanup callback.
  • A value change in one paragraph could refresh outputs bound to a different scope or interpreter group, overwriting their local edits. Updates now refresh only affected consumers, while preserving reconciliation when the same server value is received again and fallback behavior after removal.

@voidmatcha

voidmatcha commented Nov 2, 2026

Copy link
Copy Markdown
Member

I reproduced a case against a real server where an AngularObject value remains visible after restarting the interpreter and clicking "Reload from note file", even though the binding no longer exists on the server. Editing the value then changes only the UI; the update is not applied on the server and an error is returned.

The NOTE response to RELOAD_NOTE does not include the request's msgId, so it appears to skip the cache reset. Could we also reconcile the bindings with the current server state on this path?

@miinhho

miinhho commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for catching this. I confirmed that RELOAD_NOTE sends a NOTE response followed by the current AngularObjects, but the response does not carry the request’s msgId, so the client skips the cache reset. Since the replay only includes existing objects, bindings that no longer exist on the server remain cached on the client.

I’ll make the reload response follow the existing read-response path, clearing the cached bindings before applying the current server bindings, while preserving the cache for ordinary NOTE broadcasts. I’ll also add regression coverage for restarting the interpreter and then reloading the note.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@miinhho @voidmatcha